Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add GNU_PROPERTY_X86_ISA_1_NEEDED detection #535

Merged
merged 10 commits into from
Feb 9, 2025
Merged

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Feb 2, 2025

ISA extensions usage is not defined by a PEP yet.

This first implementation fails to repair the wheel if the usage of x86-64-v[2-4] is required.

The check can be disabled with --disable-isa-ext-check.

The detection being related to a declaration when building, it will not detect the requirement for binaries where the declaration is missing.

All executables built on a manylinux_2_34 image will be detected as x86-64-v2.

ISA extensions usage is not defined by a PEP yet.

This first implementation fails to repair the wheel if the usage of x86-64-v[2-4] is required.

The check can be disabled with `--disable-isa-ext-check`.

The detection being related to a declaration when building, it will not detect the requirement for binaries where the declaration is missing.

All executables built on a manylinux_2_34  image will be detected as x86-64-v2.
Copy link

codecov bot commented Feb 2, 2025

Codecov Report

Attention: Patch coverage is 93.25153% with 22 lines in your changes missing coverage. Please review.

Project coverage is 92.09%. Comparing base (5276a7b) to head (037c72a).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/auditwheel/lddtree.py 91.11% 6 Missing and 6 partials ⚠️
src/auditwheel/wheel_abi.py 91.78% 3 Missing and 3 partials ⚠️
src/auditwheel/main_lddtree.py 0.00% 3 Missing ⚠️
src/auditwheel/main_show.py 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #535      +/-   ##
==========================================
+ Coverage   92.05%   92.09%   +0.04%     
==========================================
  Files          20       22       +2     
  Lines        1321     1543     +222     
  Branches      248      291      +43     
==========================================
+ Hits         1216     1421     +205     
- Misses         65       73       +8     
- Partials       40       49       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mayeut mayeut marked this pull request as draft February 2, 2025 14:55
@mayeut mayeut marked this pull request as ready for review February 8, 2025 07:39
@mayeut mayeut requested a review from lkollar February 8, 2025 07:40
@mayeut
Copy link
Member Author

mayeut commented Feb 8, 2025

@lkollar, would you have time to review this in the coming weeks please ?

@lkollar
Copy link
Contributor

lkollar commented Feb 8, 2025

Hey @mayeut, yes, I'll take a look.

src/auditwheel/json.py Show resolved Hide resolved
src/auditwheel/json.py Outdated Show resolved Hide resolved
src/auditwheel/lddtree.py Show resolved Hide resolved
@mayeut
Copy link
Member Author

mayeut commented Feb 9, 2025

Thanks for the review @lkollar
I only pushed changes related to your comments. Will re-read one more time and merge if all is green.

@mayeut mayeut merged commit f5f83dd into pypa:main Feb 9, 2025
16 checks passed
@mayeut mayeut deleted the isa branch February 9, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants